On the todos:

Integrate the file-notify functionality so the cache can be updated whenever the underlying bibliography file changes.

Do you really mean file-notify?

Or is this necessary to avoid the update when running citar-select-ref?

Right, as it stands the cache never gets updated at all, even when calling citar-select-ref. I should change this so it re-parses the bibliography on calling citar-select-ref if the file has changed. Then we'd hook it up to filenotify so that the update happens automatically and there isn't a big wait on the next completion session.

First glance looks good.

When you think it's ready to merge to the other branch, mark it ready for review?

I'll test more a bit later.

Or if easier, I suppose you could rebase on main, and I can close the other.

Up to you.

Sure, I'll rebase on main so we don't have to do another merge dance in the future.

Except why keep track of the checksums if we need file-notify?

With new cache design, maybe we should turn on file-notify be default?

We don't need file notify, but it's nice to have. File-notify can also report spurious change events in some cases, so the checksum guards against unnecessary updates. And yes, we can turn on file-notify by default now. But we should probably have a fallback for people who can't use file-notify (maybe they're on a network file system or something?)

We'll have to fine-tune the details on this and add appropriate customization options. For example, I can imagine somebody might have a bibliography on a tramp connection and they don't want emacs to try to update it every time they start completion. Even just checking the file size and time could involve a long round-trip they'd rather not have.

Can you also take a look at the new capf and make sure it works?

I have a problem testing it.

Should just require a slight tweak to work with this.

I'm going to edit your TODO directly, to keep it all in one place.

I'll put some general notes here:

First, I tried citar-org-roam and it works without modification! I may move that back here, BTW.

Second, performance:

First, not surprisingly, this version of citar--ref-completion-tables is significantly (~5.5x) faster than citar--format-candidates (and the version in #628):

ELISP> (benchmark-run-compiled 100 (citar--ref-completion-table))
(12.98203636 43 6.759782593000001)

Alas, see my comment below on UX snappiness with large libraries: #634 (comment). In that scenario, not caching the completion candidates means it's slower overall.

Aside: maybe we should change the name to citar--ref-format-completions?

Responsiveness with large libraries

This approach has the major advantage that the "has-" indicators are always in sync. It also seems a clean architecture, with appropriate separation of concerns.

But citar-select-ref is slower, and it is noticeable with large libraries.

Try this 6000+ reference/3.2 MB file.

http://www-alg.ist.hokudai.ac.jp/~thomas/COLTBIB/colt.bib.gz

There is a noticeable pause, akin to having no cache on my ~1000 item bib.

What if we cache the completion candidates too?

This approximates that (I don't include the affixation function, but it doesn't make an obvious difference):

ELISP> (setq my/completions (citar--ref-completion-table))
#<hash-table equal 6934/8311 0x475237>
ELISP> (completing-read "test: " my/completions)

The difference is obvious (though not with my own smaller library).

I guess we'll need to decide what kind of performance users should expect, with what size libraries?

My own view is one shouldn't expect for the citar UI to be highly responsive with such large files.

But this approach and also caching the completions are not mutually exclusive, so we could always add it later if needed?

Context: I'm running these tests on a Lenovo ThinkPad X1 Carbon 7th gen laptop, with i5-8265U processor and 16 GB of RAM. Not the fastest machine, for sure, but no slouch.

We don't need file notify, but it's nice to have?

Why?

But citar-select-ref is slower, and it is noticeable with large libraries.

Thanks for that big file; I will test with it on top of own ~1000 key bibliography. I'm implementing some low-hanging fruit in terms of improving performance that should help.

However, you're right that no matter how hard we try, for large enough libraries performance will be a concern. I'd prefer not to add another cache, however, with all the accompanying headaches. Instead, I think we should expose a few options that gradually make the results less "live" but improve performance. The idea would be to maintain the current architecture, but have the "preformat" stage do more and more work.

So, for example, we currently adapt the completions to the current frame width. We could have a defcustom to specify a fixed width for completion candidates. Then the preformat can produce a single string of that width instead of a list of strings, and there's little work to be done at display-time.

An even more extreme option would make the preformat include the has:note and has:file markers, and they would only be updated when the bibliography changes or you run citar-refresh.

We'd have these options disabled by default, but document them in the Readme as "options for large bibliography files". What do you think? If you enabled all these options, I think you'd get pretty much the performance of having a candidates cache (but only a few users will need to go that far).

Why?

Without file-notify, if the bibliography changes, you will have to wait for it to be parsed the next time you call completion. People might prefer to have Emacs re-parse it automatically (perhaps when it's idle) so there's no wait.

Can you also take a look at the new capf and make sure it works?

I modified citar-capf.el to account for the new API, and when I run (citar-capf) manually in a Markdown or Org buffer it seems to return reasonable results. But I still can't get the completion to work for some reason. I've added citar-capf to completion-at-point-functions, and completion does work normally in other buffers. I'll investigate this further when I have some time. In the meantime I've pushed the small changes needed.

I think we should expose a few options that gradually make the results less "live" but improve performance. The idea would be to maintain the current architecture, but have the "preformat" stage do more and more work.

In any case, I do like the idea of incremental iteration (certainly not doing everything in this PR), in response to real user needs, but to have a clean foundation.

Do you agree this foundation is, indeed, cleaner, and easier to maintain going forward, than the current code?

So, for example, we currently adapt the completions to the current frame width. We could have a defcustom to specify a fixed width for completion candidates. Then the preformat can produce a single string of that width instead of a list of strings, and there's little work to be done at display-time.

An even more extreme option would make the preformat include the has:note and has:file markers, and they would only be updated when the bibliography changes or you run citar-refresh.

We'd have these options disabled by default, but document them in the Readme as "options for large bibliography files". What do you think? If you enabled all these options, I think you'd get pretty much the performance of having a candidates cache (but only a few users will need to go that far).

Do you imagine two boolean defcustoms? Or one with choices?

EDIT: or maybe one defcustom called something like citar-caching-strategy or some such, with the default what it is now?

Why?

Without file-notify, if the bibliography changes, you will have to wait for it to be parsed the next time you call completion. People might prefer to have Emacs re-parse it automatically (perhaps when it's idle) so there's no wait.

Right. But then would want to be when idle (or using the async library; though that would add a dependency).

Can you also take a look at the new capf and make sure it works?

I modified citar-capf.el to account for the new API, and when I run (citar-capf) manually in a Markdown or Org buffer it seems to return reasonable results. But I still can't get the completion to work for some reason. I've added citar-capf to completion-at-point-functions, and completion does work normally in other buffers. I'll investigate this further when I have some time. In the meantime I've pushed the small changes needed.

+1

Do you agree this foundation is, indeed, cleaner, and easier to maintain going forward, than the current code?

Yes, I think all the future improvements I can think of would be easier to build upon this new architecture than what we have now.

As you say, we should respond to real user needs. If we decide that we need better performance on very large files, I'm not opposed to adding new caches if needed. But this should be a last resort.

Sometimes it's easy to avoid improving performance just by adding a cache on top, but the complexity of the system increases rapidly when you do this. And if we do end up adding another cache in the end, it's good if it can be invalidated and reconstructed quickly anyway :)

Do you imagine two boolean defcustoms? Or one with choices?

I'll have to think about this more, and I think the correct design will become apparent as we go. I need to do some profiling to figure out where the bottlenecks currently are so we're not optimizing the wrong thing.

I'll have to think about this more, and I think the correct design will become apparent as we go.

It might be the defcustom values are user-oriented.

Like the current behavior results in constantly updated related resource indicators, which sacrifices some ultimate performance and responsiveness, while the other optimizations you note sacrifice the former to optimize the latter.

EDIT: maybe a defcustom like citar-update-related-resources-ui, with values of with-bib-files (for large libraries) and always (default)? Something like that would be simple and clear for users, and flexible enough, but leave room for us to iterate, including internally.

Actually, all this might be moot. With some optimization (a little hacky for now), I'm able to get it down to about 800 ms to format 7000 entries, with live has:notes and has:files indicators and adaptive column widths. And I think there are some more avenues for improvement.

Turns out ~70% of the time was being spent in garbage collection, not in string handling...

This sounds great! Just to add a user perspective, I currently always use a file exported from Zotero with my full library (ca 6500 entries), so keeping that fast is important for me. The initial parsing always takes 5-10 seconds or something (with json) which is of course unavoidable, but in case significant and slow work with all entries is done at other stages, I think the suggestion of configuring more "static" display could be good. I’ll gladly help testing when you think it is ready enough.

The initial parsing always takes 5-10 seconds or something (with json) ...

Did you ever test JSON vs BibTeX?

My hypothesis is JSON would be faster, but I've never tested.

If it is, we might note that in the docs.

I’ll gladly help testing when you think it is ready enough.

I think once Roshan pushes that commit he's working on, it should be ready to test this piece.

Actually, all this might be moot. With some optimization (a little hacky for now), I'm able to get it down to about 800 ms ...

That's great. But wiill it be fast enough for citar-select-ref, in cases like @andersjohansson'?

Now that he confirms using a very large library (I wasn't sure how common it was), we should probably make him happy :-)

Turns out ~70% of the time was being spent in garbage collection, not in string handling...

And the upshot of that? We can tune to reduce the impact?

I would be inclined to remove the file-notify functionality for now. The code is rather complex, and I can't think of a good way to have it work without locking out the UI. Without it, you get the parsing delay the first time you use a Citar command after the bibliography has changed, rather than every time it changes.

In the future I'd like to look into a way of doing the parsing in the background, but that is going to be challenging. A background emacs process would need to send the parsed data back as text, parsing which is probably just as slow as parse-bib. And apparently the Emacs multi-threading support is very immature and not suitable for serious use.

EDIT: Sorry, I'd missed your recent messages. Needless to say, I agree :)

I've pushed a new commit which changes the caching behaviour. Now calling any citar command will refresh any files if and only if needed; see the commit message. I feel this is the right default for almost all users.

I've removed the citar-cache-refresh command since it shouldn't be needed any more. For development purposes (while working on the parsing code for example) it's probably more useful to just clear the cache with (clrhash citar-cache--bibliographies).

If there is demand, we can add a mode where Citar never updates files after first reading them, unless you manually request it. This could be useful for people who have large, frequently changing bibliographies and want Citar to intentionally use outdated information. But before doing this, I'd like to take a hard look at the performance of parsing and see what the bottlenecks are.

Finally, as a small help for benchmarking, Citar now prints a message whenever it's updating a bibliography, and prints the elapsed time. If you want to benchmark the candidate formatting, which is the other performance-sensitive feature, use

(citar-get-bibliographies) ;; make sure bibliographies are in cache
(benchmark-run-compiled 10 (citar--format-candidates)) ;; Remember to divide the elapsed time by 10

Crossing-fingers, and .... merged!

I also submitted the citar-embark recipe to MELPA.

There are still a couple of little things; please take a look, including at my question above, and feel free to submit (nice and small) PRs!

Updated, and everything seems to work great!

Some feedback on real-world performance:

On my 6500 items library, with native json support:

Updating bibliography ~/bibliotek/bibliotek.json...done (3.700 seconds)
Updating bibliography ~/bibliotek/bibliotek.bib...done (10.567 seconds)

Running:

(benchmark-run-compiled 10 (citar--format-candidates))

Gives: (7.243678976 39 6.092884648). .7 seconds doesn’t seem too bad.

Thanks to the updates of org-cite-csl-activate, @andras-simonyi, one parsing step is cut out from my workflow as well.

I guess people on older computers may perhaps need to take action.

Nevertheless, my switch from citing in org-mode with the help of org-zotxt which calls the really slow Zotero server, to citing with citar feels like a great update. Citing once the library is parsed is really quick!

@andersjohansson - we have identified some performance issues in #659, which should be addressed in parsebib and in citar "soon". But it only impacts initial parsing, or when the file changes.